test(proxy): add interception path coverage for WebSocket refactor#21
test(proxy): add interception path coverage for WebSocket refactor#21andybons wants to merge 1 commit into
Conversation
Add 15 tests exercising handleConnectWithInterception through the full CONNECT+TLS interception path. These form the behavioral baseline for the upcoming ReverseProxy refactor that adds WebSocket support. Coverage: credential injection, canonical log fields, multi-request keepalive, network policy denial, transport errors, extra/remove headers, request body forwarding, large responses, status codes, response headers, X-Request-Id injection, Proxy-Authorization stripping, and HTTP methods.
|
Claude finished @andybons's task in 5m 49s —— View job Code Review
|
|
Superseded by #22 which includes these tests plus the ReverseProxy refactor and WebSocket support. |
* test(proxy): add interception path coverage for ReverseProxy refactor
Add 15 tests exercising handleConnectWithInterception through the full
CONNECT+TLS interception path. These form the behavioral baseline for
the upcoming ReverseProxy refactor that adds WebSocket support.
Coverage: credential injection, canonical log fields, multi-request
keepalive, network policy denial, transport errors, extra/remove
headers, request body forwarding, large responses, status codes,
response headers, X-Request-Id injection, Proxy-Authorization
stripping, and HTTP methods.
* test(proxy): add WebSocket upgrade test (expected to fail)
The test sends a WebSocket upgrade through the CONNECT+TLS interception
path and verifies bidirectional byte exchange after the 101 response.
Currently hangs because the manual request loop cannot handle protocol
upgrades — resp.Write blocks on the 101 response body (the persistent
WebSocket connection).
* feat(proxy): replace interception loop with ReverseProxy for WebSocket support
Replace the manual for { http.ReadRequest → transport.RoundTrip → resp.Write }
loop in handleConnectWithInterception with http.Server + httputil.ReverseProxy.
ReverseProxy natively handles WebSocket upgrades (101 Switching Protocols)
by hijacking both sides and doing bidirectional io.Copy via its built-in
switchProtocolCopier.
All existing behaviors preserved:
- Credential injection (Rewrite hook)
- Network policy and Keep policy (wrapping handler)
- LLM gateway policy (ModifyResponse via evaluateAndReplaceLLMResponse)
- Response transformers (ModifyResponse)
- Canonical log lines (ModifyResponse + ErrorHandler)
- X-Request-Id injection (Rewrite)
- Extra/remove headers, token substitution (Rewrite)
- Host gateway rewrite (Rewrite)
- Proxy-Authorization preserved from In request for token-exchange
Also addresses PR #21 review feedback:
- Fix unreachable 407 assertion in NetworkPolicyDenial test
- Remove unused BackendHostPort field from test setup
- Handle http.NewRequest errors in tests
* docs: add v0.9.0 changelog entry, remove design docs
* fix(proxy): address code review feedback on ReverseProxy refactor
1. Fix goroutine/FD leak: singleConnListener now uses ConnState to
detect StateClosed/StateHijacked and close the listener, allowing
Serve to exit. For WebSocket (hijacked), defer skips closing the
underlying connections since ReverseProxy owns them.
2. Fix credential error silently dropped: moved getCredentialsForRequest
and injectMCPCredentialsWithContext to the wrapping handler (before
ReverseProxy.ServeHTTP) so errors get an early 502 return. Resolved
credentials are passed to Rewrite via context.
3. Fix token-substituted URLs in logs: capture pre-substitution URL in
Rewrite via interceptLogURLKey context key; ModifyResponse and
ErrorHandler use it for canonical log lines instead of req.URL which
contains real tokens after substitution.
4. Fix zero Duration in policy denial logs: moved time.Now() to the top
of the wrapping handler, before any policy checks.
* fix(proxy): address third round of review feedback
1. Security: snapshot pre-injection headers in Rewrite so credential
values don't appear in canonical log RequestHeaders. Uses
interceptPreInjHeadersKey context key.
2. Bug: evaluateAndReplaceLLMResponse now returns (denied, reason) so
ModifyResponse can set Denied/DenyReason in the canonical log line.
Restores parity with the old loop's llmDenied/llmDenyReason tracking.
3. Bug: capture RequestBody in wrapping handler via captureBody before
ReverseProxy consumes it. Passed to ModifyResponse via
interceptReqBodyKey context.
4. Minor: denial/error log URLs now use req.URL.RequestURI() instead of
req.URL.Path to preserve query parameters.
* fix(proxy): add credential leak regression test, fix cred error log fields
- Add assertion in TestIntercept_CredentialInjectionCanonicalLog that
logged RequestHeaders does NOT contain the injected Authorization value
- Add RequestSize/ResponseSize to credential error log path for
consistency with other early-return paths
* fix(proxy): propagate request ID, add missing log fields in ErrorHandler
- Set X-Request-Id on req.Header in wrapping handler before calling
ReverseProxy so Rewrite preserves the same ID used in denial logs
- Add RequestSize/ResponseSize to ErrorHandler log for consistency
with other early-return paths

Summary
Adds 15 tests exercising
handleConnectWithInterceptionthrough the full CONNECT+TLS interception path. These form the behavioral baseline for the upcoming ReverseProxy refactor that adds WebSocket upgrade support.Tests added
CredentialInjectionCredentialInjectionCanonicalLogMultiRequestKeepaliveNetworkPolicyDenialNetworkPolicyDenialConnectionSurvivesTransportError502CanonicalLogFieldsExtraHeadersRemoveHeadersRequestBodyForwardedLargeResponseBodyResponseStatusCodesResponseHeadersXRequestIdInjectedProxyAuthorizationStrippedHTTPMethodsAlso includes the design doc for the WebSocket/ReverseProxy refactor at
docs/2026-04-22-websocket-reverseproxy-design.md.